Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Migration to other tap broken #17290

Open
3 tasks done
pe opened this issue May 13, 2024 · 10 comments
Open
3 tasks done

Migration to other tap broken #17290

pe opened this issue May 13, 2024 · 10 comments
Labels
bug Reproducible Homebrew/brew bug help wanted We want help addressing this

Comments

@pe
Copy link

pe commented May 13, 2024

brew doctor output

Your system is ready to brew.

Verification

  • My "brew doctor output" above says Your system is ready to brew. and am still able to reproduce my issue.
  • I ran brew update twice and am still able to reproduce my issue.
  • This issue's title and/or description do not reference a single formula e.g. brew install wget. If they do, open an issue at https://github.com/Homebrew/homebrew-core/issues/new/choose instead.

brew config output

HOMEBREW_VERSION: 4.2.21-117-gd7e81be
ORIGIN: https://github.com/Homebrew/brew
HEAD: d7e81bed359e1cbe9fb935ff40e1fd85269388fb
Last commit: 12 hours ago
Core tap HEAD: 1847a9646b873b32ed47e26c31b348fca27caf01
Core tap last commit: 14 minutes ago
Core tap JSON: 13 May 19:06 UTC
Core cask tap JSON: 13 May 19:06 UTC
HOMEBREW_PREFIX: /opt/homebrew
HOMEBREW_CASK_OPTS: []
HOMEBREW_EDITOR: micro
HOMEBREW_MAKE_JOBS: 10
HOMEBREW_SORBET_RUNTIME: set
Homebrew Ruby: 3.1.4 => /opt/homebrew/Library/Homebrew/vendor/portable-ruby/3.1.4/bin/ruby
CPU: 10-core 64-bit arm_firestorm_icestorm
Clang: 15.0.0 build 1500
Git: 2.45.0 => /opt/homebrew/bin/git
Curl: 8.4.0 => /usr/bin/curl
macOS: 14.4.1-arm64
CLT: 15.3.0.0.1.1708646388
Xcode: N/A
Rosetta 2: false

What were you trying to do (and why)?

I am trying to migrate a cask from one tap to another tap. For this, I followed Migrating a Formula to a Tap.

To reproduce, I created two repositories. And migrated a sample cask (a copy of AnyBar).

As per the instructions I

What happened (include all command output)?

Working with the migrated cask returns errors.
There are two warnings instead of one. One for a cask and one for a formula. But there is only a cask.
The error list the same tap twice.

~ > brew info example --debug --verbose
Warning: Formula pe/old-tap/example was renamed to pe/new-tap/example.
/opt/homebrew/Library/Homebrew/brew.rb (Formulary::NullLoader): loading example
Warning: Cask pe/old-tap/example was renamed to pe/new-tap/example.
Error: Cask example exists in multiple taps:
       * pe/new-tap/example
       * pe/new-tap/example

Please use the fully-qualified name (e.g. pe/new-tap/example) to refer to a specific Cask.
/opt/homebrew/Library/Homebrew/cask/cask_loader.rb:467:in `try_new'
/opt/homebrew/Library/Homebrew/vendor/bundle/ruby/3.1.0/gems/sorbet-runtime-0.5.11375/lib/types/private/methods/call_validation.rb:270:in `bind_call'
/opt/homebrew/Library/Homebrew/vendor/bundle/ruby/3.1.0/gems/sorbet-runtime-0.5.11375/lib/types/private/methods/call_validation.rb:270:in `validate_call'
/opt/homebrew/Library/Homebrew/vendor/bundle/ruby/3.1.0/gems/sorbet-runtime-0.5.11375/lib/types/private/methods/_methods.rb:277:in `block in _on_method_added'
/opt/homebrew/Library/Homebrew/cask/cask_loader.rb:567:in `block in for'
/opt/homebrew/Library/Homebrew/cask/cask_loader.rb:566:in `each'
/opt/homebrew/Library/Homebrew/cask/cask_loader.rb:566:in `for'
/opt/homebrew/Library/Homebrew/cask/cask_loader.rb:517:in `load'
/opt/homebrew/Library/Homebrew/cli/named_args.rb:179:in `block in load_formula_or_cask'
/opt/homebrew/Library/Homebrew/api.rb:201:in `with_no_api_env_if_needed'
/opt/homebrew/Library/Homebrew/vendor/bundle/ruby/3.1.0/gems/sorbet-runtime-0.5.11375/lib/types/private/methods/call_validation.rb:270:in `bind_call'
/opt/homebrew/Library/Homebrew/vendor/bundle/ruby/3.1.0/gems/sorbet-runtime-0.5.11375/lib/types/private/methods/call_validation.rb:270:in `validate_call'
/opt/homebrew/Library/Homebrew/vendor/bundle/ruby/3.1.0/gems/sorbet-runtime-0.5.11375/lib/types/private/methods/_methods.rb:277:in `block in _on_method_added'
/opt/homebrew/Library/Homebrew/cli/named_args.rb:138:in `load_formula_or_cask'
/opt/homebrew/Library/Homebrew/cli/named_args.rb:131:in `block in to_formulae_and_casks_and_unavailable'
/opt/homebrew/Library/Homebrew/cli/named_args.rb:130:in `map'
/opt/homebrew/Library/Homebrew/cli/named_args.rb:130:in `to_formulae_and_casks_and_unavailable'
/opt/homebrew/Library/Homebrew/cmd/info.rb:153:in `print_info'
/opt/homebrew/Library/Homebrew/vendor/bundle/ruby/3.1.0/gems/sorbet-runtime-0.5.11375/lib/types/private/methods/call_validation.rb:270:in `bind_call'
/opt/homebrew/Library/Homebrew/vendor/bundle/ruby/3.1.0/gems/sorbet-runtime-0.5.11375/lib/types/private/methods/call_validation.rb:270:in `validate_call'
/opt/homebrew/Library/Homebrew/vendor/bundle/ruby/3.1.0/gems/sorbet-runtime-0.5.11375/lib/types/private/methods/_methods.rb:277:in `block in _on_method_added'
/opt/homebrew/Library/Homebrew/cmd/info.rb:106:in `run'
/opt/homebrew/Library/Homebrew/vendor/bundle/ruby/3.1.0/gems/sorbet-runtime-0.5.11375/lib/types/private/methods/call_validation.rb:270:in `bind_call'
/opt/homebrew/Library/Homebrew/vendor/bundle/ruby/3.1.0/gems/sorbet-runtime-0.5.11375/lib/types/private/methods/call_validation.rb:270:in `validate_call'
/opt/homebrew/Library/Homebrew/vendor/bundle/ruby/3.1.0/gems/sorbet-runtime-0.5.11375/lib/types/private/methods/_methods.rb:277:in `block in _on_method_added'
/opt/homebrew/Library/Homebrew/brew.rb:92:in `<main>'

It can only be uninstalled with the full name.

~ > brew uninstal example
Warning: Formula pe/old-tap/example was renamed to pe/new-tap/example.
Warning: Cask pe/old-tap/example was renamed to pe/new-tap/example.
Error: Cask example exists in multiple taps:
       * pe/new-tap/example
       * pe/new-tap/example

Please use the fully-qualified name (e.g. pe/new-tap/example) to refer to a specific Cask.
~ > brew uninstal pe/new-tap/example
==> Uninstalling Cask example
==> Backing App 'AnyBar.app' up to '/opt/homebrew/Caskroom/example/0.2.3/AnyBar.app'
==> Removing App '/Applications/AnyBar.app'
==> Purging files for version 0.2.3 of Cask example

It can only be installed with the full name.

~ > brew install pe/old-tap/example
==> Auto-updating Homebrew...
Adjust how often this is run with HOMEBREW_AUTO_UPDATE_SECS or disable with
HOMEBREW_NO_AUTO_UPDATE. Hide these hints with HOMEBREW_NO_ENV_HINTS (see `man brew`).
Warning: Formula pe/old-tap/example was renamed to pe/new-tap/example.
==> Downloading https://github.com/tonsky/AnyBar/releases/download/0.2.3/AnyBar-0.2.3.zip
Already downloaded: /Users/philippe.eberli/Library/Caches/Homebrew/downloads/c2696299dd799237cb088ec14ae2b8ead2d3d84b6bcb1eb7dfa52eef9219318d--AnyBar-0.2.3.zip
==> Installing Cask example
==> Moving App 'AnyBar.app' to '/Applications/AnyBar.app'
🍺  example was successfully installed!
~ > brew uninstal pe/new-tap/example
==> Uninstalling Cask example
==> Backing App 'AnyBar.app' up to '/opt/homebrew/Caskroom/example/0.2.3/AnyBar.app'
==> Removing App '/Applications/AnyBar.app'
==> Purging files for version 0.2.3 of Cask example
~ > brew install pe/new-tap/example
==> Downloading https://github.com/tonsky/AnyBar/releases/download/0.2.3/AnyBar-0.2.3.zip
Already downloaded: /Users/philippe.eberli/Library/Caches/Homebrew/downloads/c2696299dd799237cb088ec14ae2b8ead2d3d84b6bcb1eb7dfa52eef9219318d--AnyBar-0.2.3.zip
==> Installing Cask example
==> Moving App 'AnyBar.app' to '/Applications/AnyBar.app'
🍺  example was successfully installed!

What did you expect to happen?

The migrated cask can be used without errors and without having to specify the full name (=excluding the tap)

Step-by-step reproduction instructions (by running brew commands)

~ ❯ brew tap pe/old-tap
==> Tapping pe/old-tap
Cloning into '/opt/homebrew/Library/Taps/pe/homebrew-old-tap'...
remote: Enumerating objects: 14, done.
remote: Counting objects: 100% (14/14), done.
remote: Compressing objects: 100% (11/11), done.
remote: Total 14 (delta 1), reused 13 (delta 0), pack-reused 0
Receiving objects: 100% (14/14), done.
Resolving deltas: 100% (1/1), done.
Tapped (16 files, 9.8KB).
~ ❯ brew install example
==> Downloading https://github.com/tonsky/AnyBar/releases/download/0.2.3/AnyBar-0.2.3.zip
Already downloaded: /Users/philippe.eberli/Library/Caches/Homebrew/downloads/c2696299dd799237cb088ec14ae2b8ead2d3d84b6bcb1eb7dfa52eef9219318d--AnyBar-0.2.3.zip
==> Installing Cask example
==> Moving App 'AnyBar.app' to '/Applications/AnyBar.app'
🍺  example was successfully installed!


⇒ migrating the example cask from old-tap to new-tap. See commits in the description.


~ ❯ brew tap pe/new-tap
==> Tapping pe/new-tap
Cloning into '/opt/homebrew/Library/Taps/pe/homebrew-new-tap'...
remote: Enumerating objects: 11, done.
remote: Counting objects: 100% (11/11), done.
remote: Compressing objects: 100% (9/9), done.
remote: Total 11 (delta 0), reused 11 (delta 0), pack-reused 0
Receiving objects: 100% (11/11), done.
Tapped 1 cask (16 files, 9.9KB).
@pe pe added the bug Reproducible Homebrew/brew bug label May 13, 2024
@carlocab
Copy link
Member

carlocab commented May 13, 2024

I think your tap_migrations.json in pe/old-tap should be

{
    "example": "pe/new-tap/example"
}

@pe
Copy link
Author

pe commented May 13, 2024

I think your tap_migrations.json in pe/old-tap should be

{
    "example": "pe/new-tap/example"
}

Looking at other examples I don't think so: https://github.com/brewsci/homebrew-science/blob/5d14d2fda3934c0eb8243c1cbdb9fb4621ec110b/tap_migrations.json

I tried it with pe/homebrew-old-tap@cc86591. Same error.

@carlocab carlocab added the help wanted We want help addressing this label May 13, 2024
@pe
Copy link
Author

pe commented May 22, 2024

I think I found the source of the issue. The same cask is found twice. Once in the new tap and once in the old but mapped to the new tap.

If I inspect loaders from this code:

loaders = Tap.select { |tap| tap.installed? && !tap.core_cask_tap? }
.filter_map { |tap| super("#{tap}/#{token}", warn:) }
.select { |tap| tap.path.exist? }

I get this:

[
#<Cask::CaskLoader::FromNameLoader:0x0000000125cac788 @token="example", @path=#<Pathname:/opt/homebrew/Library/Taps/pe/homebrew-new-tap/Casks/example.rb>, @tap=#<Tap:0x0000000122de3898 @user="pe", @repo="new-tap", @name="pe/new-tap", @full_name="pe/homebrew-new-tap", @path=#<Pathname:/opt/homebrew/Library/Taps/pe/homebrew-new-tap>, @git_repo=#<GitRepository:0x0000000122e0f6f0 @pathname=#<Pathname:/opt/homebrew/Library/Taps/pe/homebrew-new-tap>>, @alias_dir=#<Pathname:/opt/homebrew/Library/Taps/pe/homebrew-new-tap/Aliases>, @alias_files=[], @alias_table={}, @formula_renames={}, @tap_migrations={}, @potential_formula_dirs=[#<Pathname:/opt/homebrew/Library/Taps/pe/homebrew-new-tap/Formula>, #<Pathname:/opt/homebrew/Library/Taps/pe/homebrew-new-tap/HomebrewFormula>, #<Pathname:/opt/homebrew/Library/Taps/pe/homebrew-new-tap>], @formula_dir=#<Pathname:/opt/homebrew/Library/Taps/pe/homebrew-new-tap>, @formula_files=[], @formula_files_by_name={}, @cask_renames={}, @cask_dir=#<Pathname:/opt/homebrew/Library/Taps/pe/homebrew-new-tap/Casks>, @cask_files=[#<Pathname:/opt/homebrew/Library/Taps/pe/homebrew-new-tap/Casks/example.rb>], @cask_files_by_name={"example"=>#<Pathname:/opt/homebrew/Library/Taps/pe/homebrew-new-tap/Casks/example.rb>}>>,
#<Cask::CaskLoader::FromNameLoader:0x0000000125caa410 @token="example", @path=#<Pathname:/opt/homebrew/Library/Taps/pe/homebrew-new-tap/Casks/example.rb>, @tap=#<Tap:0x0000000122de3898 @user="pe", @repo="new-tap", @name="pe/new-tap", @full_name="pe/homebrew-new-tap", @path=#<Pathname:/opt/homebrew/Library/Taps/pe/homebrew-new-tap>, @git_repo=#<GitRepository:0x0000000122e0f6f0 @pathname=#<Pathname:/opt/homebrew/Library/Taps/pe/homebrew-new-tap>>, @alias_dir=#<Pathname:/opt/homebrew/Library/Taps/pe/homebrew-new-tap/Aliases>, @alias_files=[], @alias_table={}, @formula_renames={}, @tap_migrations={}, @potential_formula_dirs=[#<Pathname:/opt/homebrew/Library/Taps/pe/homebrew-new-tap/Formula>, #<Pathname:/opt/homebrew/Library/Taps/pe/homebrew-new-tap/HomebrewFormula>, #<Pathname:/opt/homebrew/Library/Taps/pe/homebrew-new-tap>], @formula_dir=#<Pathname:/opt/homebrew/Library/Taps/pe/homebrew-new-tap>, @formula_files=[], @formula_files_by_name={}, @cask_renames={}, @cask_dir=#<Pathname:/opt/homebrew/Library/Taps/pe/homebrew-new-tap/Casks>, @cask_files=[#<Pathname:/opt/homebrew/Library/Taps/pe/homebrew-new-tap/Casks/example.rb>], @cask_files_by_name={"example"=>#<Pathname:/opt/homebrew/Library/Taps/pe/homebrew-new-tap/Casks/example.rb>}>>
]

The only difference is the object_id.

Changing the above code like so fixes the issue.

loaders = Tap.select { |tap| tap.installed? && !tap.core_cask_tap? }
             .filter_map { |tap| super("#{tap}/#{token}", warn:) }
             .select { |tap| tap.path.exist? }
             .uniq{ |cask| cask.tap } # this line was added by me

But my knowledge of this codebase is none and my ruby is very limited. I can't judge if this is a good idea or not.

How to proceed? Should I just create a PR and we'll discuss there? Would you want any tests for this change? Where? Are there other places where this is also necessary?

@gromgit
Copy link
Member

gromgit commented May 23, 2024

I can't judge if this is a good idea or not.

My Ruby-fu is limited too, but I'm pretty sure that:

  • the whole point of migrations is that the formula/cask no longer exists in the old tap
  • taps are processed in alpha-sorted order, so if your old tap sorts before the new one (e.g. pe/tap1 -> pe/tap2), your fix will pick the wrong tap

@MikeMcQuaid
Copy link
Member

  • the whole point of migrations is that the formula/cask no longer exists in the old tap

Yes, this. You should delete the formula/cask from the old file or it won't work as expected.

@MikeMcQuaid
Copy link
Member

How to proceed? Should I just create a PR and we'll discuss there?

Yes, this would be best.

Would you want any tests for this change? Where?

Ideally the corresponding _spec.rb file to the source file(s) you edit. That can wait until we've validated the approach with a draft PR, though.

@pe
Copy link
Author

pe commented May 23, 2024

taps are processed in alpha-sorted order, so if your old tap sorts before the new one (e.g. pe/tap1 -> pe/tap2), your fix will pick the wrong tap

I don't think that the order matters because both casks are identical (except for the object_id). The cask from the old tap was already migrated to the new one, that's why they are identical.

  • the whole point of migrations is that the formula/cask no longer exists in the old tap

Yes, this. You should delete the formula/cask from the old file or it won't work as expected.

I did delete it in the old tap and get this error nevertheless. See What were you trying to do (and why)?.

How to proceed? Should I just create a PR and we'll discuss there?

Yes, this would be best.

OK. Will do.

@apainintheneck
Copy link
Contributor

loaders = Tap.select { |tap| tap.installed? && !tap.core_cask_tap? }
.filter_map { |tap| super("#{tap}/#{token}", warn:) }
.select { |tap| tap.path.exist? }

I suspect what's happening here is that we're getting identical cask loaders because the call to super evaluates cask renames and migrations so it's possible that the original tap that the cask was migrated from is returning the cask loader for the new tap.

def self.try_new(ref, warn: false)
ref = ref.to_s
return unless (token_tap_type = CaskLoader.tap_cask_token_type(ref, warn:))
token, tap, = token_tap_type
new("#{tap}/#{token}")
end

The super call leads to the Cask::CaskLoader::FromTapLoader.try_new method which calls the Cask::CaskLoader.tap_cask_token_type method. Normally it just returns the cask token, tap and type specified but if the cask has been renamed or migrated it returns the new token, tap and type. Since they are identical, just removing duplicates by path makes sense here. Tap would also work but path is a bit more specific.

Ideally, we'd figure out how to refactor this so that this doesn't happen in this sort of situation but I'm not really sure what that would look like.

@pe
Copy link
Author

pe commented May 24, 2024

While playing around, I just noticed that the same error also applies to formulae, not only to casks.

Step-by-step reproduction instructions (by running brew commands)

https://github.com/pe/homebrew-old-tap/blob/5a0e91e527c5962096e62fa277c896a1c285faef/tap_migrations.json#L3
https://github.com/pe/homebrew-new-tap/blob/d755222b381ccbea385f7884b97b9b8b9f19429e/Formula/examplef.rb#L1

~ ❯ brew install examplef --verbose --debug
Warning: Formula pe/old-tap/examplef was renamed to pe/new-tap/examplef.
Error: Formulae found in multiple taps:
       * pe/new-tap/examplef
       * pe/new-tap/examplef

Please use the fully-qualified name (e.g. pe/new-tap/examplef) to refer to a specific formula.
/opt/homebrew/Library/Homebrew/formulary.rb:837:in `try_new'
/opt/homebrew/Library/Homebrew/formulary.rb:1227:in `block in loader_for'
/opt/homebrew/Library/Homebrew/formulary.rb:1226:in `each'
/opt/homebrew/Library/Homebrew/formulary.rb:1226:in `loader_for'
/opt/homebrew/Library/Homebrew/formulary.rb:1008:in `factory'
/opt/homebrew/Library/Homebrew/cli/parser.rb:709:in `block in formulae'
/opt/homebrew/Library/Homebrew/cli/parser.rb:705:in `each'
/opt/homebrew/Library/Homebrew/cli/parser.rb:705:in `filter_map'
/opt/homebrew/Library/Homebrew/cli/parser.rb:705:in `formulae'
/opt/homebrew/Library/Homebrew/cli/parser.rb:363:in `parse'
/opt/homebrew/Library/Homebrew/abstract_command.rb:53:in `initialize'
/opt/homebrew/Library/Homebrew/vendor/bundle/ruby/3.3.0/gems/sorbet-runtime-0.5.11385/lib/types/private/abstract/declare.rb:37:in `new'
/opt/homebrew/Library/Homebrew/vendor/bundle/ruby/3.3.0/gems/sorbet-runtime-0.5.11385/lib/types/private/abstract/declare.rb:37:in `block in declare_abstract'
/opt/homebrew/Library/Homebrew/brew.rb:90:in `<main>'

The exact same code also exists here:

loaders = Tap.select { |tap| tap.installed? && !tap.core_tap? }
.filter_map { |tap| super("#{tap}/#{name}", warn:) }
.select { |tap| tap.path.exist? }

@MikeMcQuaid
Copy link
Member

@reitermarkus Could you take a look at this if you get a chance as you did a bunch of migration work recently? Thanks ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Reproducible Homebrew/brew bug help wanted We want help addressing this
Projects
None yet
Development

No branches or pull requests

5 participants